Skip to content

chore: remove usage of tokensChainsCache from useMusdConversionStatus#28644

Merged
juanmigdr merged 1 commit intomainfrom
chore/remove-usage-tokensChainsCache-from-earn-flow
Apr 10, 2026
Merged

chore: remove usage of tokensChainsCache from useMusdConversionStatus#28644
juanmigdr merged 1 commit intomainfrom
chore/remove-usage-tokensChainsCache-from-earn-flow

Conversation

@juanmigdr
Copy link
Copy Markdown
Member

@juanmigdr juanmigdr commented Apr 10, 2026

Description

Removes the usage of selectERC20TokensByChain (which reads from tokensChainsCache) in useMusdConversionStatus. This is part of a broader effort to eliminate all usages of tokensChainsCache from the codebase so it can eventually be removed from TokenListController.

Previously, getTokenData inside the hook's useEffect looked up a token's symbol and name from the full tokensChainsCache via a useSelector + ref pattern. This required subscribing to the entire chain-indexed token list on every render.

The replacement uses selectSingleTokenByAddressAndChainId called directly against store.getState() at the moment the transaction event fires — the same point-in-time store access pattern already used in this hook for getTransactionPayQuotes. Since metamaskPay.tokenAddress is always a token the user holds in their wallet, it is tracked in TokensController.allTokens, making the tokensChainsCache lookup redundant.

Also removes the now-unnecessary safeToChecksumAddress import (previously used to handle both checksummed and lowercase address lookups, which selectSingleTokenByAddressAndChainId handles internally), and drops the tokensCacheRef ref pattern along with the useSelector / react-redux import.

Changelog

CHANGELOG entry: remove usage of tokensChainsCache from useMusdConversionStatus

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/ASSETS-2956

Manual testing steps

Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Medium Risk
Moderate risk because it changes how token metadata is resolved for mUSD conversion toasts/metrics (switching data sources and lookup behavior), which could alter displayed symbols and analytics properties in edge cases.

Overview
useMusdConversionStatus no longer reads tokensChainsCache via react-redux/selectERC20TokensByChain; it now pulls token symbol/name on-demand from store.getState() using selectSingleTokenByAddressAndChainId.

Tests are updated to mock the new selector-based lookup, remove the old chain token-cache setup (including lowercase/checksum fallback coverage), and adjust the fallback case to treat missing wallet tokens as "Token".

Reviewed by Cursor Bugbot for commit 9de0418. Bugbot is set up for automated code reviews on this repo. Configure here.

@juanmigdr juanmigdr requested a review from a team as a code owner April 10, 2026 09:44
@github-actions github-actions bot added size-M risk-low Low testing needed · Low bug introduction risk labels Apr 10, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeTrade
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: low
  • AI Confidence: 82%
click to see 🤖 AI reasoning details

E2E Test Selection:
The changes are confined to two files in the Earn hooks directory:

  1. useMusdConversionStatus.ts: Refactored token data lookup from selectERC20TokensByChain (tokenListController) to selectSingleTokenByAddressAndChainId (tokensController). This simplifies how token symbol/name is retrieved during mUSD conversion transaction monitoring. The change removes the React-Redux useSelector dependency and instead reads directly from the Redux store via the selector. No user-visible behavior changes - only the internal data source for token metadata changes.

  2. useMusdConversionStatus.test.ts: Updated unit tests to match the new implementation.

The hook is used in EarnTransactionMonitor which monitors mUSD conversion transactions and shows toast notifications. The Earn/staking feature is most closely related to SmokeTrade (which covers native ETH staking flows initiated from the wallet actions menu).

Risk is low because:

  • This is a pure internal refactor with no API/behavior changes
  • Unit tests are updated to cover the new implementation
  • No navigation, modal, or critical path changes
  • The selector selectSingleTokenByAddressAndChainId is already widely used across the codebase (confirmations, predictions, etc.)
  • No E2E tests specifically test mUSD conversion status toasts

SmokeTrade is selected as a conservative measure since the Earn staking feature falls under that category, and the mUSD conversion flow is part of the Earn ecosystem.

Performance Test Selection:
The change is a refactor of a token data selector within a hook that monitors transaction status. It doesn't affect UI rendering performance, list rendering, app startup, or any performance-critical paths. The switch from useSelector/tokenListController to selectSingleTokenByAddressAndChainId is a minor internal change with no performance implications.

View GitHub Actions results

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.14%. Comparing base (0c26cf4) to head (9de0418).
⚠️ Report is 25 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #28644      +/-   ##
==========================================
+ Coverage   82.13%   82.14%   +0.01%     
==========================================
  Files        4942     4949       +7     
  Lines      129947   130065     +118     
  Branches    28977    29002      +25     
==========================================
+ Hits       106732   106844     +112     
- Misses      15917    15924       +7     
+ Partials     7298     7297       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sonarqubecloud
Copy link
Copy Markdown

@juanmigdr juanmigdr enabled auto-merge April 10, 2026 10:08
@juanmigdr
Copy link
Copy Markdown
Member Author

@metamaskbot update-mobile-fixture

@github-actions
Copy link
Copy Markdown
Contributor

E2E Fixture Validation — Schema is up to date
15 value mismatches detected (expected — fixture represents an existing user).
View details

Copy link
Copy Markdown
Contributor

@Matt561 Matt561 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unable to test at the moment. There's an issue preventing fetching of Relay quotes for the conversion flow. Approving since the changes seem straightforward to unblock. 👍

@juanmigdr juanmigdr added this pull request to the merge queue Apr 10, 2026
Merged via the queue into main with commit aafd5e0 Apr 10, 2026
183 of 188 checks passed
@juanmigdr juanmigdr deleted the chore/remove-usage-tokensChainsCache-from-earn-flow branch April 10, 2026 16:59
@github-actions github-actions bot locked and limited conversation to collaborators Apr 10, 2026
@metamaskbot metamaskbot added the release-7.74.0 Issue or pull request that will be included in release 7.74.0 label Apr 10, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-7.74.0 Issue or pull request that will be included in release 7.74.0 risk-low Low testing needed · Low bug introduction risk size-M team-assets

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants